-
Notifications
You must be signed in to change notification settings - Fork 21
[O2B-1506] LHCfills run duration filter #2038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2038 +/- ##
==========================================
+ Coverage 45.58% 45.63% +0.05%
==========================================
Files 1031 1031
Lines 17172 17190 +18
Branches 3123 3130 +7
==========================================
+ Hits 7827 7845 +18
Misses 9345 9345 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
23dbb7b to
11c1f9d
Compare
fd1be9f to
2c6fee8
Compare
This reverts commit 695622b.
…ith the query/reset logic. Just set the value afterwards
5c9fecd to
950022d
Compare
…ture/O2B-1505/lhcfills-beam-duration-filter
…ature/O2B-1506/lhcfills-run-duration-filter
…ature/O2B-1506/lhcfills-run-duration-filter
…moved, SBDuration filter frontend fixed
…ature/O2B-1506/lhcfills-run-duration-filter
…DO validate tests
…ature/O2B-1506/lhcfills-run-duration-filter update from upstream
…ature/O2B-1506/lhcfills-run-duration-filter update from upstream
…ature/O2B-1506/lhcfills-run-duration-filter update from upstream
…fills DTO. QueryBuilder WhereAssociationQueryBuilder has been changed to handle associations with an OR operator. Added tests
| * @param {TextComparisonFilterModel} runDurationFilterModel runDurationFilterModel | ||
| * @returns {Component} the text field | ||
| */ | ||
| export const runDurationFilter = (runDurationFilterModel) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This I think could me made into a generic function given it is the same as beamDurationFilter
| // Include 00:00:00 = 0 = null AND everything above 00:00:00 which is more or less than 0. | ||
| queryBuilder.whereAssociation('statistics', 'runsCoverage').applyOperator(runDuration.operator, 0); | ||
| queryBuilder.whereAssociation('statistics', 'runsCoverage').applyOperator('=', null); | ||
| } else if (Number(runDuration.limit) === 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Operators > and < are not handled here I think properly when limit is 0. They will get caught in the 1st else if and be compared with null which won't work with > and <. Need another condition statement if > or < and limit is 0 then apply operator on 0. And add the extra test cases.
| [this.column]: operation, | ||
| }, | ||
| }); | ||
| // Check if this include association already exists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure at the moment whether this is the best solution to the be able to join 2 applyOperators. Maybe that is not solution at all and could modify apply operator that if object is passed i.e raw sequalize operator logic then you can pass in all in one go to the method instead of twice and this extra changes to be able to do twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be possible to modify the _op, applyOperator, greaterThan etc functions to take into account multiple values to set an OR in the end but I'm not sure if that is any better. If using raw Sequalize it is possible to not modify the greaterThan etc... but _op and applyOperator would still need changes
I have a JIRA ticket
Notable changes for users:
Notable changes for developers:
Changes made to the database:
-None.